Skip to content

Support project documentation with direct Markdown input #3478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Mar 3, 2025
Merged

Support project documentation with direct Markdown input #3478

merged 14 commits into from
Mar 3, 2025
+573 −29

Conversation

ciyer
Copy link
Contributor

@ciyer ciyer commented Jan 10, 2025 •
edited
Loading

Separate the project documentation support from the ck-editor upgrade in #3390.

Screenshots

UI changes compared to initial version

Display text if there is no documentation

image

Show edit view in a modal dialog

image image

/deploy

@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from a79784a to a465c67 Compare January 10, 2025 13:17
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 10, 2025 13:17 — with GitHub Actions Inactive
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3478.dev.renku.ch

@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from a465c67 to 2d76097 Compare January 10, 2025 16:35
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 10, 2025 16:35 — with GitHub Actions Inactive
@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from 5d924b5 to 52e620d Compare January 10, 2025 17:43
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 10, 2025 17:43 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 13, 2025 10:10 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 14, 2025 11:10 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 15, 2025 10:20 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 16, 2025 13:31 — with GitHub Actions Inactive
@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from 5dd97f9 to 2aa8034 Compare January 17, 2025 09:46
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 17, 2025 09:49 — with GitHub Actions Inactive
@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from 2aa8034 to e5afcce Compare January 17, 2025 10:38
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 17, 2025 10:38 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 17, 2025 12:27 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 17, 2025 12:44 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 January 17, 2025 12:56 — with GitHub Actions Inactive
leafty
leafty reviewed Jan 17, 2025
View reviewed changes
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some small comments below.

@lorenzo-cavazzi lorenzo-cavazzi marked this pull request as ready for review January 17, 2025 14:24
@lorenzo-cavazzi lorenzo-cavazzi requested a review from a team as a code owner January 17, 2025 14:24
@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from aa4c2a6 to c82a37b Compare January 17, 2025 14:46
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 February 27, 2025 16:18 — with GitHub Actions Inactive
@ciyer ciyer marked this pull request as ready for review February 28, 2025 08:27
leafty
leafty reviewed Feb 28, 2025
View reviewed changes
leafty
leafty requested changes Feb 28, 2025
View reviewed changes
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some comments here which need to be fixed.

client/src/features/ProjectPageV2/ProjectPageContent/Documentation/Documentation.tsx Outdated
<div>
<span
className={cx(
isCloseToLimit && "text-danger",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I find this disconcerting, maybe warning color would be more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the color chosen by the designer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I mean that there is no difference between being just under the limit and over the limit.

Screenshot 2025-02-28 at 16 52 52

This looks like it will reject the update but in fact it will accept it. There should be a visual difference between 4995/5000 and 5600/5000.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

It's the same here as below the limit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other solution is to not do this closeToLimit logic and only do red for over the limit. Showing red for an acceptable input is not OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have chosen to do the last one (show red if the word count is greater than the limit).

@ciyer ciyer mentioned this pull request Feb 28, 2025
Closed
@ciyer ciyer temporarily deployed to renku-ci-ui-3478 February 28, 2025 15:06 — with GitHub Actions Inactive
@ciyer ciyer requested a review from leafty February 28, 2025 15:31
@ciyer ciyer force-pushed the ciyer/project-docu-minimal branch from fc9f12a to 83a3e9a Compare February 28, 2025 16:34
leafty
leafty approved these changes Mar 1, 2025
View reviewed changes
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ciyer ciyer merged commit 6bb36c1 into main Mar 3, 2025
23 checks passed
@ciyer ciyer deleted the ciyer/project-docu-minimal branch March 3, 2025 09:19
@RenkuBot
Copy link
Contributor

RenkuBot commented Mar 3, 2025

Tearing down the temporary RenkuLab deplyoment for this PR.

@leafty leafty mentioned this pull request Mar 7, 2025
Closed
@ciyer ciyer mentioned this pull request Mar 7, 2025
Closed
leafty pushed a commit that referenced this pull request Apr 22, 2025
@ciyer @olloz26 @leafty
feat: support project documentation (#3478)
3935265 
---------

Co-authored-by: olloz26 <olloz@nexus.ethz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leafty leafty leafty approved these changes

Assignees

@leafty leafty

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ciyer @RenkuBot @leafty @lorenzo-cavazzi @olloz26